Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove project from resource processing template replacement #45

Merged
merged 2 commits into from
Apr 9, 2024
Merged

Remove project from resource processing template replacement #45

merged 2 commits into from
Apr 9, 2024

Conversation

lukebemish
Copy link

@lukebemish lukebemish commented Mar 16, 2024 •
edited
Loading

The template-expanding done in processResources currently contains:

filesMatching(['META-INF/mods.toml']) {
    expand replaceProperties + [project: project]
}

This causes issues with configuration caching - in addition to those already caused by neogradle - as it uses the project object within template expansion; to reach the eventual goal of working configuration caching in a userdev environment, this should be resolved. Specifically, the error is (if you disable the neogradle features that themselves cause failures):

luke@luke-kubuntu:~/src/test/MDK$ ./gradlew build --configuration-cache
To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/8.6/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
Daemon will be stopped at the end of the build 
Reusing configuration cache.
> Task :processResources FAILED

FAILURE: Build failed with an exception.

* Where:
Build file '/home/luke/src/test/MDK/build.gradle' line: 122

* What went wrong:
Execution failed for task ':processResources'.
> Cannot reference a Gradle script object from a Groovy closure as these are not supported with the configuration cache.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

BUILD FAILED in 3s
1 actionable task: 1 executed
Configuration cache entry reused.

Luckily, this expansion isn't actually used anywhere - so the issue can be resolved by only using replaceProperties without the added project expansion.

@sciwhiz12 sciwhiz12 added the bug Something isn't working label Mar 30, 2024
sciwhiz12
sciwhiz12 approved these changes Mar 30, 2024
View reviewed changes
Copy link
Member

@sciwhiz12 sciwhiz12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. Developers who want to reference other properties from their Gradle project in their mods.toml should add them to replaceProperties, so it's clear where all these values come from.

As a side-note, the formatting of that replaceProperties is atrocious: aligned properties, but there's also unaligned properties after those aligned ones. Makes it a bit difficult to see what properties are available, since at a glance you'd only recognize the ones that are aligned.

I'll defer merging for an hour or so, in case you want to fixup the alignment of that. 😄

@lukebemish
Copy link
Author

Fixed up the alignment

@lukebemish lukebemish requested a review from sciwhiz12 April 7, 2024 19:43
@sciwhiz12 sciwhiz12 merged commit 83959d5 into neoforged:main Apr 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sciwhiz12 sciwhiz12 sciwhiz12 approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lukebemish @sciwhiz12